cc @blaginin

Thanks @shruti2522 !

Hey! I’m a bit worried that now to understand the test you’ll need to look into two places: open the test file and also search for the snapshot file… that’s why in the example pr (#15165) I used inline snapshots.

do you think we should use the same approach here?

Hey! I’m a bit worried that now to understand the test you’ll need to look into two places: open the test file and also search for the snapshot file… that’s why in the example pr (#15165) I used inline snapshots.

do you think we should use the same approach here?

Hey @blaginin, you are right about this, at first I tried implementing the inline snapshots only, but I ran into errors while running tests for run_and_compare_query in user_defined/user_defined_plan.rs, since multiple tests are using it ,only the first one works because insta doesn't allow duplicate inline snapshots by default. I tried allow_duplicates!(), but it gets a bit tricky with async functions. So, I used snapshot files instead because they make it much easier to run multiple tests using the same query fn, also it makes it much easier to update expected outputs in the future and eliminates the use of #[rustfmt::skip] in some places. Maybe we can discuss some alternative approach to implement inline snapshots 🤔

I tried allow_duplicates!(), but it gets a bit tricky with async functions

Can you please explain more on this? I tried modifying async run_and_compare_query and this worked for me:

So, I used snapshot files instead because they make it much easier to run multiple tests using the same query fn, also it makes it much easier to update expected outputs in the future and eliminates the use of #[rustfmt::skip] in some places.

Just to give a bit of context, we had a discussion about inline / file snapshots here: #13672 (comment) and #13672 (comment).

To me personally, it feels like we can use snapshot files, but the snapshot itself should give enough information to understand the context. For example, in CLI, you can see program input and tests, and the snapshot file itself doesn't contain a lot of logic. Here I'm not sure if that's the case for ALL the test fixtures? Maybe some of them can be inline?

I tried allow_duplicates!(), but it gets a bit tricky with async functions

Can you please explain more on this? I tried modifying async run_and_compare_query and this worked for me:

> So, I used snapshot files instead because they make it much easier to run multiple tests using the same query fn, also it makes it much easier to update expected outputs in the future and eliminates the use of #[rustfmt::skip] in some places.

Just to give a bit of context, we had a discussion about inline / file snapshots here: #13672 (comment) and #13672 (comment).

To me personally, it feels like we can use snapshot files, but the snapshot itself should give enough information to understand the context. For example, in CLI, you can see program input and tests, and the snapshot file itself doesn't contain a lot of logic. Here I'm not sure if that's the case for ALL the test fixtures? Maybe some of them can be inline?

Yes, it worked. I was using insta::allow_duplicates! like a function call:

 insta::allow_duplicates!(|| {
        insta::with_settings!({
            description => description,
        }, {
            insta::assert_snapshot!(actual, @r###"
            +-------------+---------+
            | customer_id | revenue |
            +-------------+---------+
            | paul        | 300     |
            | jorge       | 200     |
            | andy        | 150     |
            +-------------+---------+
            "###);
        });
    })();

On changing it to this it worked

 insta::allow_duplicates! {
        insta::with_settings!({
            description => description,
        }, {
            insta::assert_snapshot!(actual, @r###"
            +-------------+---------+
            | customer_id | revenue |
            +-------------+---------+
            | paul        | 300     |
            | jorge       | 200     |
            | andy        | 150     |
            +-------------+---------+
        "###);
        });
    }

Thanks a lot @blaginin and @alamb ❤️

Thanks @shruti2522 and @blaginin 🙏